Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-71337] Fix configuration not being saved. #16

Merged
merged 1 commit into from
May 25, 2023

Conversation

mrmateo
Copy link

@mrmateo mrmateo commented May 25, 2023

Not 100% sure what caused it but appears to have been added when config was converted from being global. Updating to more recent conventions appears to have fixed it (ex: setting variables through constructor). Also had to add inline to config for the sub-boolean option.

Testing done

Testing updating configs from existing values, adding new config on initial setup, and that setting via the config.xml directly still parsed correctly.

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira

Not 100% sure what caused it but appears to have been added when config was converted from being global. Updating to more recent conventions appears to have fixed it (ex: setting variables through constructor). Also had to add inline to config for the sub-boolean option.
@mrmateo
Copy link
Author

mrmateo commented May 25, 2023

@lucianonet - this fixes the config save issue. Can you give me some more info about the issue you're seeing with CasC to see if it is related?

EDIT: Re-reading your question from the other commit -- I think that yes, that old PR did break your existing CasC configuration. This version (2.3.1) should have been a breaking version instead of a simple bugfix version.

Here is an example of the CasC config that works with the recently security-fixed version - https://github.com/jenkinsci/keycloak-plugin/blob/master/src/test/resources/org/jenkinsci/plugins/casc.yaml :

jenkins:
  securityRealm:
    keycloak:
      keycloakJson: |-
        {
          "realm": "master",
          "auth-server-url": "https://keycloak.example.com/auth/",
          "ssl-required": "external",
          "resource": "ci-example-com",
          "credentials": {
            "secret": "secret-secret-secret"
          },
          "confidential-port": 0
        }
      keycloakRespectAccessTokenTimeout: true
      keycloakValidate: false

So instead of being unclassified it is now part of the security realm configuration area. I will look to see how I can mark this keycloak update as being breaking.

@mrmateo mrmateo merged commit 4b2efa8 into jenkinsci:master May 25, 2023
@mrmateo mrmateo deleted the bugfix/JENKINS-71337 branch May 25, 2023 21:43
@mrmateo mrmateo added the bug label May 25, 2023
@lucianonet
Copy link

@lucianonet - this fixes the config save issue. Can you give me some more info about the issue you're seeing with CasC to see if it is related?

EDIT: Re-reading your question from the other commit -- I think that yes, that old PR did break your existing CasC configuration. This version (2.3.1) should have been a breaking version instead of a simple bugfix version.

Here is an example of the CasC config that works with the recently security-fixed version - https://github.com/jenkinsci/keycloak-plugin/blob/master/src/test/resources/org/jenkinsci/plugins/casc.yaml :

jenkins:
  securityRealm:
    keycloak:
      keycloakJson: |-
        {
          "realm": "master",
          "auth-server-url": "https://keycloak.example.com/auth/",
          "ssl-required": "external",
          "resource": "ci-example-com",
          "credentials": {
            "secret": "secret-secret-secret"
          },
          "confidential-port": 0
        }
      keycloakRespectAccessTokenTimeout: true
      keycloakValidate: false

So instead of being unclassified it is now part of the security realm configuration area. I will look to see how I can mark this keycloak update as being breaking.

@mrmateo Thank you. It's working now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants